Skip to content
This repository was archived by the owner on Mar 9, 2022. It is now read-only.

Add container logging support. #56

Merged
Random-Liu merged 2 commits into from
Jun 2, 2017
Merged

Add container logging support. #56

Random-Liu merged 2 commits into from
Jun 2, 2017

Conversation

@Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented May 30, 2017

Based on #53, #52 and #48. Only the last 2 commits are new.

This PR added container logging support.

@Random-Liu
Copy link
Member Author

Random-Liu commented May 30, 2017

With this PR, we could pass more CRI validation test now:

Summarizing 14 Failures:

[Fail] [k8s.io] Security Context runtime should support container with security context [It] runtime should support setting Capability 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:252

[Fail] [k8s.io] Networking runtime should support networking [It] runtime should support port mapping with host port and container port [Conformance] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/networking.go:206

[Fail] [k8s.io] Security Context runtime should support NamespaceOption in the Security Context [It] runtime should support HostIpc is false [security context] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:315

[Fail] [k8s.io] Security Context runtime should support NamespaceOption in the Security Context [It] runtime should support HostIpc is true [security context] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:315

[Fail] [k8s.io] Security Context runtime should support container with security context [It] runtime should support Privileged is true [security context] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:539

[Fail] [k8s.io] Security Context runtime should support NamespaceOption in the Security Context [It] runtime should support HostNetwork is true [security context] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/security_context.go:603

[Fail] [k8s.io] Networking runtime should support networking [It] runtime should support DNS config [Conformance] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/networking.go:165

[Fail] [k8s.io] Streaming runtime should support streaming interfaces [It] runtime should support portforward [Conformance] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/streaming.go:231

[Fail] [k8s.io] Streaming runtime should support streaming interfaces [It] runtime should support exec [Conformance] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/streaming.go:127

[Fail] [k8s.io] Streaming runtime should support streaming interfaces [It] runtime should support attach [Conformance] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/streaming.go:183

[Fail] [k8s.io] Security Context runtime should support container with security context [It] runtime should support RunAsUser [security context] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:315

[Fail] [k8s.io] Security Context runtime should support NamespaceOption in the Security Context [It] runtime should support HostPID [security context] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/util.go:214

[Fail] [k8s.io] Security Context runtime should support container with security context [It] runtime should support RunAsUserName [security context] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:315

[Fail] [k8s.io] Container runtime should support basic operations on container [It] runtime should support execSync [Conformance] 
/home/lantaol/workspace/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/container.go:315

Ran 36 of 36 Specs in 150.726 seconds
FAIL! -- 22 Passed | 14 Failed | 0 Pending | 0 Skipped --- FAIL: TestE2ECRI (150.73s)
FAIL

Ginkgo ran 1 suite in 2m30.796156486s
Test Suite Failed
exit status 1

Most failures are expected, I'll add execSync soon to help run more test.

One weird thing is that network is not working. @xlgao-zju

@Random-Liu
Copy link
Member Author

Random-Liu commented May 30, 2017

@xlgao-zju OK. It's working.

The host network and host port is not working.

Host port not working is expected, but host network should work. Will take a look.

@Random-Liu Random-Liu added this to the v0.1.0-rc2 milestone May 30, 2017
@Random-Liu Random-Liu force-pushed the add-container-logging branch from 3163c02 to 99c82a0 Compare May 30, 2017 21:51
@Random-Liu
Copy link
Member Author

Random-Liu commented May 30, 2017

Fixed the code, and Hostnetwork test also passed now.

@Random-Liu Random-Liu force-pushed the add-container-logging branch from 99c82a0 to c6041d5 Compare May 31, 2017 17:59
@Random-Liu Random-Liu force-pushed the add-container-logging branch from c6041d5 to c3a55e7 Compare May 31, 2017 18:56
mikebrow
mikebrow approved these changes Jun 2, 2017
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM just rebase to fix conflict...

@mikebrow mikebrow added the lgtm label Jun 2, 2017
@Random-Liu Random-Liu mentioned this pull request Jun 2, 2017
Closed
42 tasks
@Random-Liu
Copy link
Member Author

Random-Liu commented Jun 2, 2017 •
edited
Loading

Will rebase and merge this PR. But we should be aware of that after switching to the new containerd client:

  1. We'll still need the cri log format decorating logic;
  2. We'll not need the agent factory because in the containerd client, because the routine creation logic with be in WithXXX functions, and will not actually run in mock client.

Random-Liu added 2 commits June 2, 2017 16:36
@Random-Liu
Add container logging support.
e657e1e 
Signed-off-by: Lantao Liu <[email protected]>
@Random-Liu
Add unit test
69fcf97 
Signed-off-by: Lantao Liu <[email protected]>
@Random-Liu Random-Liu force-pushed the add-container-logging branch from c3a55e7 to 69fcf97 Compare June 2, 2017 16:52
@Random-Liu Random-Liu merged commit 42131ac into containerd:master Jun 2, 2017
@Random-Liu Random-Liu deleted the add-container-logging branch June 2, 2017 17:11
lanchongyizu pushed a commit to lanchongyizu/cri-containerd that referenced this pull request Sep 3, 2017
@Random-Liu
Merge pull request containerd#56 from Random-Liu/add-container-logging
71734f3 
Add container logging support.
dcantah pushed a commit to dcantah/cri that referenced this pull request Apr 17, 2020
Merge pull request containerd#56 from katiewasnothere/vpci_device
bf88280 
GPU Assigned Device Support
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mikebrow mikebrow mikebrow approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@yujuhong yujuhong

@mikebrow mikebrow

Labels

cncf-cla: yes lgtm

Projects

None yet

Milestone

v0.1.0-alpha.1

Development

Successfully merging this pull request may close these issues.

4 participants

@Random-Liu @mikebrow @yujuhong @k8s-ci-robot